Skip to content

remove math dependency; speedups #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 24, 2018
Merged

Conversation

dhalbert
Copy link
Contributor

@dhalbert dhalbert commented May 24, 2018

  1. For pirkey, I needed to remove math to be able to freeze a number of modules. adafruit_dotstar uses math.ceil(). I removed that dependency and redid those calculations in specialized ways, which probably sped them up.
  2. Replace enumerate() with range() in fill(). This speeds up fill() by 25% or so.
  3. Don't bother trimming the four-tuple with brightness, since the value isn't used anyway.

In general I am trying to avoid generating garbage. If people were to use integer values a lot instead of tuples, we could preallocate an rgb bytearray and store into it instead of converting the integer value to a tuple.

I experimented with pre-fetching the pixel order values in advance and storing them in instance variables. That would improve fill by about 4%. I didn't bother with that right now.

Tagging @mcscope, who may be interested.

@dhalbert dhalbert requested review from tannewt and kattni May 24, 2018 03:37
@dhalbert
Copy link
Contributor Author

dhalbert commented May 24, 2018

@tannewt the change from range() to enumerate() tracks the same change in NeoPixel. Not sure why it was changed: enumerate() fetches the values as well as the indices, but discards the values. Do you remember why? Am I missing something?

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enumerate use is encouraged by pylint when its range(len(foo)). I think this file evolved away from it recently and now its not suggested.

Looks good to me!

@tannewt tannewt merged commit af25424 into adafruit:master May 24, 2018
tannewt pushed a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request May 25, 2018
@dhalbert dhalbert deleted the speedups branch March 25, 2022 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants